-
Notifications
You must be signed in to change notification settings - Fork 759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create Dockerfile #542
base: uinverse
Are you sure you want to change the base?
Create Dockerfile #542
Conversation
Create dockerfile due to industry pressure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok to me.
The cat image seems like a little bit of bloat |
I assure you it is essential.
…On Tue, Nov 23, 2021 at 9:51 AM, skyracer2012 ***@***.***> wrote:
The cat image seems like a little bit of bloat
—
You are receiving this because you authored the thread.
Reply to this email directly, [view it on GitHub](#542 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AJ6H6YIJCG23JNIYFN3RLN3UNPIA5ANCNFSM5HAM6TCQ).
Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub).
|
looks good to me. |
Please include 30+ page analysis of container runtimes so that we can acheive preliminary stage 0.1 approval form the architecture review board. |
|
||
WORKDIR ./src | ||
|
||
ADD https://images.pexels.com/photos/4587959/pexels-photo-4587959.jpeg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't. If I remove the cat picture everything stops working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we gonna resolve this issue?
Company management wants us to scale into cloud but the cat image is blocking it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PurHur please see my comment below regarding a possible resolution for this issue. bd44545#r901147477
|
||
WORKDIR ./src | ||
|
||
ADD https://images.pexels.com/photos/4587959/pexels-photo-4587959.jpeg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please replace the hyperlink with a locally stored jpeg
file to appease corporate demand for cloud scalability while avoiding cross-hosting headaches as well as, God-forbid, 404 missing file issues if pexels.com goes offline.
Here is the cat image, enhanced and upscaled using Machine Learning (i.e., future-proofed for devices using 16k resolution):
https://we.tl/t-okV1rNmg3d
You should add this image to the base/root of the project and then change line 7 of the Dockerfile as follows:
- ADD https://images.pexels.com/photos/4587959/pexels-photo-4587959.jpeg
+ ADD cat.jpeg
Note that ADD
is no longer necessary as it is the same as COPY
when using locally-referenced files. Therefore, the same line can be rewritten as follows:
COPY cat.jpeg .
In summary, it is my expert opinion that the proposed Dockerfile in full should appear like so:
FROM java:8
COPY . /var/www/java
WORKDIR ./src
COPY cat.jpeg .
RUN ./main/java/com/seriouscompany/business/java/fizzbuzz/packagenamingpackage/impl/Main.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for your work so we doing huge steps forward.
Another big problem here is the copyright of the cat image. We dont have a written contract to use this image in our software and we should avoid getting sued for 120m damages.
So i propose to replace the cat image with an ai generated cat image that has no copyright.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that a copyright free image would be the best option here.
However, we should be cognizant to the developing field of AI-rights wherein the ownership of AI-generated works of art may belong to the AI, or perhaps the creator of said AI, and not necessarily the public domain.
We should acquire an AI-generated cat image and revisit this issue of legality in 20 years from today's date. Put this on your calendars.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you prove the ownership and sign a contract that is added to the docker image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also happy to fly out to our Enterprise Headquarters in the corporate private jet to provide additional proof.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that is enough. We should use a picture of Kimchi.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we just need @Tylersuard to update his Dockerfile and resubmit his PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe relying on pexels is not sustainable; I'd commit the image into directly the repository instead.
But I do agree we should investigate why the Dockerfile is not working unless we add more cats to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I do agree we should investigate why the Dockerfile is not working unless we add more cats to it.
We did the research a year ago. This project needs to cut some costs so we cant do it again. One image of kimchi would not add much bloat to the current state of our goal.
|
||
WORKDIR ./src | ||
|
||
ADD https://images.pexels.com/photos/4587959/pexels-photo-4587959.jpeg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PurHur please see my comment below regarding a possible resolution for this issue. bd44545#r901147477
@Tylersuard do you have a time horizon for finalizing this pull request? Your work looks great and the copyright issue with the cat image is solved. If you could exchange the image with a picture of Kimchi that would be great. This is the last thing that blocks the Company to migrate to the cloud. |
This opens the door to introduce a dockerfile builder, which is important, because we might want to add any number of cat pictures in the future (at least one). |
Please include more cat breeds for localization |
I actually got feedback of our consulting firm. The review is positive and we got an OK for merging this! 🎉 Who is in charge merging this great work? I guess @Mikkeren isnt working at the company anymore. |
We clearly have issues locating a dependency (a person authorised to merge this PR). Instead of a service locator, we should try dependency injection for future PRs. |
Create dockerfile due to industry pressure.